Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Authorization Grant Flow without Redirect URI #2484

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Conversation

strehle
Copy link
Member

@strehle strehle commented Sep 12, 2023

Add test for this hidden feature thus not to run into a regression

Add test for this hidden feature thus not to run into a regression
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/186021972

The labels on this github issue will be updated when the story is started.

@strehle strehle requested a review from a team September 12, 2023 10:56
@@ -82,7 +82,7 @@
<entry key="secret" value="loginsecret"/>
<entry key="scope" value="openid,oauth.approvals"/>
<entry key="authorized-grant-types" value="client_credentials,authorization_code"/>
<entry key="redirect-uri" value="http://localhost/**,http://localhost:7000/**"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this change? Is this required for the added test?

Copy link
Member Author

@strehle strehle Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it mainly is the revert of my change https://github.com/cloudfoundry/uaa/pull/2403/files#diff-d02078cfeded6b34f4eb0a68b222758f944db9bbe2fa1227249a9c782718cf09R85

the login client is used in our internal forked scenario and it was broken. Because UAA has this hidden feature of redirect uris. It means if you have only one redirect uri defined you can omit the parameter redirect_uri in authorize request.
If you have an array in the client you cannot use this feature anymore therefore removed the 2nd entry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and create an explicit test for this hidden feature

Map<String, String> body = IntegrationTestUtils.getAuthorizationCodeTokenMap(serverRunning,
testAccounts,
"login",
"loginsecret",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this client has only one redirect uri therefore you can omit it.


try {
IntegrationTestUtils.getAuthorizationCodeTokenMap(serverRunning, testAccounts, "app", "appclientsecret",
testAccounts.getUserName(), testAccounts.getPassword(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the app client has 2 entries for redirect-uri thus here it does not work.

These tests simply should help to know what we have already, nothing new

@strehle strehle added this to the 76.21.0 milestone Sep 12, 2023
@strehle strehle merged commit 4e04770 into develop Sep 12, 2023
20 checks passed
@strehle strehle deleted the testAuthorize branch September 12, 2023 17:51
@cf-gitbot cf-gitbot added delivered accepted Accepted the issue and removed delivered labels Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted the issue
Projects
Development

Successfully merging this pull request may close these issues.

3 participants